test: add compressed audio socwatch test#1345
Open
ekurdybx wants to merge 2 commits intothesofproject:mainfrom
Open
test: add compressed audio socwatch test#1345ekurdybx wants to merge 2 commits intothesofproject:mainfrom
ekurdybx wants to merge 2 commits intothesofproject:mainfrom
Conversation
ekurdybx
changed the title
ekurdybx
force-pushed
the
compressed_audio_socwatch_test
branch
2 times, most recently
from
8a46815 to
70150f4
Compare
ekurdybx
force-pushed
the
compressed_audio_socwatch_test
branch
6 times, most recently
from
c9edb0c to
f1455fb
Compare
ekurdybx
force-pushed
the
compressed_audio_socwatch_test
branch
4 times, most recently
from
b3ad6c8 to
a5626ac
Compare
ekurdybx
force-pushed
the
compressed_audio_socwatch_test
branch
from
a5626ac to
0bcbd9a
Compare
Signed-off-by: Emilia Kurdybelska <emiliax.kurdybelska@intel.com>
ekurdybx
force-pushed
the
compressed_audio_socwatch_test
branch
from
0bcbd9a to
7883e5c
Compare
ekurdybx
changed the title
ekurdybx
changed the title
ekurdybx
force-pushed
the
compressed_audio_socwatch_test
branch
2 times, most recently
from
028929a to
d2d4f57
Compare
ekurdybx
requested review from
a team,
golowanow,
lgirdwood and
marc-hb
as code owners
redzynix
reviewed
Apr 17, 2026
| print(f"Incorrect value: {pc_state} time % was {value}, expected {expected_value}") | ||
| failures += 1 | ||
|
|
||
| return 0 if failures == 0 else 1 |
Contributor
There was a problem hiding this comment.
It's cool but I think keeping it easier to read would be better.
Suggested change
| return 0 if failures == 0 else 1 | |
| if failures == 0: | |
| return 0 | |
| return 1 |
redzynix
approved these changes
Apr 17, 2026
ekurdybx
force-pushed
the
compressed_audio_socwatch_test
branch
from
d2d4f57 to
cc6155f
Compare
Add test that plays mp3 audio using cplay, and check PC states with Socwatch Signed-off-by: Emilia Kurdybelska <emiliax.kurdybelska@intel.com>
ekurdybx
force-pushed
the
compressed_audio_socwatch_test
branch
from
cc6155f to
6154da5
Compare
marc-hb
reviewed
Apr 17, 2026
Collaborator
marc-hb
left a comment
marc-hb
left a comment
There was a problem hiding this comment.
Very clean and easy to read! Minor suggestions below
| with open(pc_states_file, encoding="utf-8") as file: | ||
| for line in file: | ||
| m = pattern.match(line) | ||
| if m: |
Collaborator
There was a problem hiding this comment.
"early return" pattern
Suggested change
| if m: | |
| if not m: | |
| continue |
Same as you already did below but saves a lot more indentation (and mental load).
| print(f"Incorrect value: {pc_state} time % was {value}, expected {expected_value}") | ||
| failures += 1 | ||
|
|
||
| if failures: |
Collaborator
There was a problem hiding this comment.
Suggested change
| if failures: | |
| return failures |
Then you may not want the main script to exit the number of failures too because some values may have special meaning - but for sure this function has nothing better to return than the number of failures.
| pattern = re.compile(r'^PC(\d+)\s*,\s*(\d+(?:\.\d+)?)\s*,\s*(\d+(?:\.\d+)?)\s*$') | ||
| failures = 0 | ||
|
|
||
| with open(pc_states_file, encoding="utf-8") as file: |
Collaborator
There was a problem hiding this comment.
You could move the with open() to the caller and save even more indentation.
|
|
||
|
|
||
| def analyze_pc_states(pc_states_file, expected_results): | ||
| pattern = re.compile(r'^PC(\d+)\s*,\s*(\d+(?:\.\d+)?)\s*,\s*(\d+(?:\.\d+)?)\s*$') |
Collaborator
There was a problem hiding this comment.
Regexps are notoriously cryptic. What really helps in such cases is to give a few input examples in a comment.
| check_cplay_command() | ||
| { | ||
| dlogi "${play_command[@]}" | ||
| if ! "${play_command[@]}"; then |
Collaborator
There was a problem hiding this comment.
Style nit:
Suggested change
| if ! "${play_command[@]}"; then | |
| "${play_command[@]}" || | |
| die ... |
| expected_results="{\"PC10.2\":$pc10_threshold}" | ||
|
|
||
| # Analyze if the % of the time spent in given PC state was as expected | ||
| if python3 "$SCRIPT_HOME"/tools/analyze-pc-states.py "$pc_states_file" "$expected_results"; then |
Collaborator
There was a problem hiding this comment.
chmod a+x analyze-pc-states.py
I'm afraid sof-test is not going to work on Windows anytime soon!
Suggested change
| if python3 "$SCRIPT_HOME"/tools/analyze-pc-states.py "$pc_states_file" "$expected_results"; then | |
| if "$SCRIPT_HOME"/tools/analyze-pc-states.py "$pc_states_file" "$expected_results"; then |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add test that plays MP3 sound and performs Socwatch analysis to check if dut enters and stays in PC10 state for the expected % of time (default expected value: 80% in PC10.2 state)
Note: this test will only pass when: